Skip to content

refactor: extract shared event log codec#68

Merged
ThomasK33 merged 3 commits into
mainfrom
grill-docs-cyte
Apr 29, 2026
Merged

refactor: extract shared event log codec#68
ThomasK33 merged 3 commits into
mainfrom
grill-docs-cyte

Conversation

@ThomasK33
Copy link
Copy Markdown
Member

Summary

  • Extracted a shared persisted event-log codec under src/storage/eventLogCodec.ts for file size checks, JSONL parsing, EventRecordSchema validation, contiguous sequence validation, and file reads.
  • Rewired live event-log hydration, offline replay, and record export to use the shared codec while keeping append-time and replay-specific behavior in their existing modules.
  • Added focused storage codec tests and updated affected host/replay/export tests and mocks.
  • Added dogfood proof bundle at dogfood/issue-59-event-log-codec/ with CLI outputs, copied event log/session manifests, screenshot, asciicast, and WebM artifacts.

User-facing / automation-facing behavior

  • Public CLI JSON envelopes, event-record shapes, replay/export outputs, and artifact manifests remain unchanged.
  • Missing event-log file policy remains caller-specific: offline replay treats missing logs as empty, while record export still surfaces missing logs as an export error.
  • Event-log validation errors now use neutral event-log wording from the shared codec.

Validation

  • npx vitest run test/unit/storage/eventLogCodec.test.ts test/unit/host/eventLog.test.ts test/unit/host/replay.test.ts test/unit/replay/offlineReplay.test.ts test/unit/commands/record-export.test.ts test/unit/export/webm.test.ts
  • npx vitest run --maxWorkers=1 test/integration/event-log.test.ts
  • npm run typecheck
  • npm run lint
  • npm run format:check
  • npm run verify

Dogfood

  • Proof bundle: dogfood/issue-59-event-log-codec/
  • Isolated home recorded in dogfood/issue-59-event-log-codec/home.txt
  • Contiguous event-log proof: dogfood/issue-59-event-log-codec/event-log-check.json
  • Screenshot proof: dogfood/issue-59-event-log-codec/screenshot-9-reference-dark.png
  • Asciicast: dogfood/issue-59-event-log-codec/recording.cast
  • WebM: dogfood/issue-59-event-log-codec/recording.webm

Closes #59.


📋 Implementation Plan

Plan: Issue #59 — Extract shared event-log codec

Context and evidence

GitHub issue: coder/agent-tty#59 — “Extract the event log codec used by append and replay”.

Current verified state:

  • src/host/eventLog.ts owns live event-log append behavior and also duplicates persisted-log hydration helpers:
    • local MAX_EVENT_LOG_SIZE = 50 * 1024 * 1024
    • JSONL parsing
    • EventRecordSchema validation
    • contiguous seq validation
  • src/host/replay.ts owns replay input construction and currently duplicates persisted-log reader concerns:
    • exported MAX_EVENT_LOG_SIZE = 50 * 1024 * 1024
    • readEventLogRecords() file reader
    • JSONL parsing
    • EventRecordSchema validation
    • contiguous replay-event sequence validation
  • Replay/export callsites currently consume readEventLogRecords() from src/host/replay.ts:
    • src/replay/offlineReplay.ts
    • src/cli/commands/record-export.ts
  • buildReplayInput() is also used by:
    • src/host/hostMain.ts
    • src/export/webm.ts
    • offline replay/export tests
  • Existing affected tests include:
    • test/unit/host/eventLog.test.ts
    • test/unit/host/replay.test.ts
    • test/integration/event-log.test.ts
    • related offline replay / record export tests where import mocks change
  • Domain language resolved during grilling:
    • Event Log is the append-only history of a Session's terminal output, user inputs, control actions, and lifecycle events.
    • It is canonical replay truth for reconstructing session state and generating artifacts.

Decisions from grilling

  1. Use a narrow shared codec.

    • The codec owns persisted event-log concerns only:
      • size limit
      • JSONL parsing
      • event-record schema validation
      • contiguous seq validation
      • file-to-EventRecord[] reading
    • EventLog keeps live append sequencing, payload-specific append overloads, buffering, rollback, and disk writes.
    • replay.ts keeps manifest validation and ReplayInput construction.
  2. Place the codec at src/storage/eventLogCodec.ts.

    • The behavior is about persisted event-log storage, not live-host orchestration or protocol messaging.
  3. Move imports directly to the codec.

    • Do not keep compatibility re-exports from src/host/replay.ts.
    • Internal TypeScript imports are not public API, and direct imports make ownership clear.
  4. Standardize codec-level errors around neutral event-log wording.

    • JSONL parsing errors should be line-based:
      • event log line N must be valid JSON
      • event log line N must match EventRecordSchema
    • Already-loaded array validation errors should be record-index-based, not line-based:
      • event log record N must match EventRecordSchema
    • Use 1-based non-empty-line ordinals for JSONL lines, preserving current behavior after blank/whitespace-only lines are filtered.
    • Use zero-based array indexes for already-loaded records, matching current buildReplayInput() behavior.
    • Shared sequence/size examples:
      • first event log seq must be 0
      • event log seq values must increase by 1 without gaps
      • event log file exceeds size limit (... bytes, max ...)
    • Replay-specific wording should remain only for replay-specific invariants such as manifest/session mismatch, dimensions, and targetSeq bounds.
  5. Share in-memory event validation too.

    • buildReplayInput() should call the codec for already-loaded EventRecord[] validation, then apply replay-specific logic.
  6. Missing event-log files remain caller policy.

    • readEventLogRecords() should throw ENOENT for missing files.
    • withOfflineReplayRenderer() should continue treating missing event logs as empty replay.
    • record export should continue surfacing missing event logs as an error.
  7. No ADR is needed.

    • This is an internal, reversible module-boundary refactor and does not meet the bar for a hard-to-reverse architectural decision.

Proposed implementation

Phase 1 — Add the shared codec

Create src/storage/eventLogCodec.ts with a small, focused API:

export const MAX_EVENT_LOG_SIZE = 50 * 1024 * 1024;

export function assertEventLogSize(size: number): void;
export function parseEventLogContent(content: string): EventRecord[];
export function validateEventRecords(events: readonly unknown[]): EventRecord[];
export async function readEventLogRecords(filePath: string): Promise<EventRecord[]>;

Implementation details:

  • Use EventRecordSchema from src/protocol/schemas.ts.
  • Use existing invariant() from src/util/assert.ts.
  • Add defensive assertions:
    • filePath must be non-empty in readEventLogRecords().
    • size must be an integer and non-negative in assertEventLogSize().
    • event arrays should be validated from readonly unknown[] to EventRecord[].
  • Preserve current JSONL semantics:
    • split by \n
    • trim lines
    • ignore blank / whitespace-only lines
    • report parsing failures with 1-based non-empty-line numbering, matching current behavior
  • readEventLogRecords() should:
    • stat(filePath)
    • call assertEventLogSize(stats.size)
    • readFile(filePath, 'utf8')
    • call parseEventLogContent(content)
    • let ENOENT and other filesystem errors propagate

Dependency boundary:

  • src/storage/eventLogCodec.ts should only import low-level dependencies such as src/protocol/schemas.ts, Node filesystem modules, and src/util/assert.ts.
  • It must not import src/host/*, src/replay/*, CLI commands, or export modules; this keeps the storage codec reusable and avoids dependency cycles.

Phase 2 — Rewire live event-log hydration

Update src/host/eventLog.ts:

  • Remove local persisted-log helpers that move to the codec:
    • local MAX_EVENT_LOG_SIZE
    • parseEventLogLine()
    • assertContiguousSequence()
    • parseEventLogContent()
  • Import from src/storage/eventLogCodec.ts:
    • assertEventLogSize
    • parseEventLogContent
  • Do not import MAX_EVENT_LOG_SIZE into src/host/eventLog.ts unless the implementation still needs it directly; avoid unused imports.
  • Keep append-time payload validation and EventRecordSchema validation local to EventLog.append().
  • In EventLog.open():
    • continue opening with append mode (open(filePath, 'a')) so missing files are created for the writer path
    • continue using fileHandle.stat()
    • replace inline size invariant with assertEventLogSize(fileStats.size)
    • hydrate existing content via parseEventLogContent(existingContent)
    • continue deriving nextSeq from the hydrated buffer

Phase 3 — Rewire replay/export readers

Update src/host/replay.ts:

  • Remove local file-reading and JSONL parsing helpers:
    • local MAX_EVENT_LOG_SIZE
    • readEventLogRecords()
    • local event-record parser
    • local contiguous sequence assertion
  • Import validateEventRecords() from src/storage/eventLogCodec.ts.
  • Keep buildReplayInput() in replay.ts.
  • In buildReplayInput():
    • validate sessionId, manifest, dimensions, and targetSeq as before
    • replace events.map(parseEventRecord) + local sequence check with validateEventRecords(events)
    • keep replay-specific target sequence behavior unchanged:
      • empty events require resolved target seq -1
      • non-empty events require targetSeq >= 0 and targetSeq <= lastSeq

Update direct imports for file readers/constants:

  • src/replay/offlineReplay.ts
    • import readEventLogRecords from ../storage/eventLogCodec.js
    • keep buildReplayInput from ../host/replay.js
  • src/cli/commands/record-export.ts
    • import readEventLogRecords from ../../storage/eventLogCodec.js
  • Tests that use MAX_EVENT_LOG_SIZE or readEventLogRecords should import from src/storage/eventLogCodec.js.
  • Update mocks in tests that currently mock src/host/replay.js just to stub readEventLogRecords():
    • readEventLogRecords mocks should move to src/storage/eventLogCodec.js.
    • buildReplayInput mocks should stay on src/host/replay.js.

Phase 4 — Rehome and narrow tests

Add test/unit/storage/eventLogCodec.test.ts as the canonical codec contract test suite.

Cover at least:

  • assertEventLogSize() accepts exactly MAX_EVENT_LOG_SIZE and rejects MAX_EVENT_LOG_SIZE + 1.
  • readEventLogRecords() rejects oversized logs with the canonical event-log message.
  • parseEventLogContent() returns [] for empty content.
  • parseEventLogContent() / readEventLogRecords() parse valid JSONL.
  • Blank and whitespace-only lines are ignored.
  • Blank lines before malformed JSON preserve the current 1-based non-empty-line ordinal behavior.
  • Malformed JSONL lines are rejected with event log line N must be valid JSON.
  • Invalid event-record shapes from JSONL are rejected with event log line N must match EventRecordSchema.
  • Invalid already-loaded records are rejected with event log record N must match EventRecordSchema using zero-based indexes.
  • First seq other than 0 is rejected.
  • Sequence gaps, duplicate seq values, and decreasing seq values are rejected.
  • validateEventRecords() validates already-loaded arrays and rejects non-contiguous arrays.
  • Missing files propagate ENOENT from readEventLogRecords(); assert the error code, not the filesystem message.

Keep existing tests but narrow their responsibilities:

  • test/unit/host/eventLog.test.ts
    • still proves EventLog.open() hydrates existing logs and derives the next sequence correctly
    • still proves append behavior, rollback, buffer limits, and getEventsSince() behavior
    • update expected codec-level error strings as needed
  • test/unit/host/replay.test.ts
    • keep buildReplayInput() tests for manifest/dimension/target-seq semantics
    • update sequence-validation expectations to canonical event-log wording if needed
    • move file-reader tests to codec tests
  • test/integration/event-log.test.ts
    • unchanged intent: prove CLI action sequences produce monotonic valid event records
  • Offline replay / record export tests
    • update imports/mocks only as needed to reflect direct codec ownership

Phase 5 — Quality gates

Run targeted automated validation first:

npm run test:unit -- test/unit/storage/eventLogCodec.test.ts
npm run test:unit -- test/unit/host/eventLog.test.ts test/unit/host/replay.test.ts
npm run test:unit -- test/unit/replay/offlineReplay.test.ts test/unit/commands/record-export.test.ts test/unit/export/webm.test.ts
npm run test:integration -- test/integration/event-log.test.ts

Then run static checks:

npm run typecheck
npm run lint
npm run format:check

The offline replay, record export, and WebM tests are required targeted checks because import ownership changes across those boundaries.

For final confidence before commit/PR, prefer:

npm run verify

or, if mise is available:

mise run ci

Dogfooding plan

Because this is a CLI/storage refactor that affects replay/export readers, perform a lightweight isolated dogfood smoke after automated tests pass.

Create an isolated absolute home, for example:

export AGENT_TTY_HOME="$(mktemp -d)"

Use local development invocation rather than public skill examples, and make the session exit before replay/export so the dogfood definitely exercises the persisted offline replay path:

npx tsx src/cli/main.ts create --json -- /bin/sh
npx tsx src/cli/main.ts type <session-id> "printf 'hello from codec dogfood\\n'" --json
npx tsx src/cli/main.ts send-keys <session-id> Enter --json
npx tsx src/cli/main.ts type <session-id> "exit" --json
npx tsx src/cli/main.ts send-keys <session-id> Enter --json
npx tsx src/cli/main.ts wait <session-id> --idle-ms 500 --timeout 5000 --json
npx tsx src/cli/main.ts snapshot <session-id> --json
npx tsx src/cli/main.ts record export <session-id> --format asciicast --json
npx tsx src/cli/main.ts destroy <session-id> --json

Save a reviewer proof bundle under dogfood/issue-59-event-log-codec/ containing:

  • commands.sh with the exact commands run
  • command JSON outputs
  • copied events.jsonl
  • copied session.json or manifest excerpt
  • exported asciicast artifact, if generated
  • screenshot JSON/artifact and/or WebM only if renderer dependencies are available
  • notes explaining any unavailable screenshot/video recording capability

Dogfood acceptance:

  • the event log exists and has contiguous seq values
  • offline replay consumer (snapshot) succeeds after the session exits or is otherwise offline replay eligible
  • record export succeeds from the same event log
  • no command mutates the real ~/.agent-tty

Acceptance criteria

  • The event-log size limit is defined in exactly one implementation location.
  • EventLog.open() hydration uses the shared codec for size and JSONL/content validation.
  • Replay/export readers use the shared codec for file reading and validation.
  • buildReplayInput() uses the shared codec for already-loaded event arrays.
  • JSONL parsing exists in one place.
  • EventRecordSchema validation for loaded/persisted event records is centralized in the codec, while append-time constructed-record validation remains local to EventLog.append().
  • Contiguous sequence validation exists in one place.
  • Missing event-log files still propagate from the codec and remain caller-specific policy.
  • Event record shapes remain unchanged.
  • Public CLI JSON envelopes, replay/export output formats, and artifact manifests remain unchanged.
  • Targeted unit/integration tests pass.
  • Dogfood proof bundle is created or an explicit environment limitation is documented.

Risks and mitigations

  • Risk: accidentally changing append semantics.
    • Mitigation: keep append payload validation, record construction, buffering, rollback, and writes in EventLog.
  • Risk: changing CLI JSON/output formats while moving imports.
    • Mitigation: do not alter protocol schemas, result shapes, artifact manifests, or command output code.
  • Risk: tests assert old replay-specific wording.
    • Mitigation: update tests to canonical event-log wording only for codec-level validation failures.
  • Risk: mock churn hides integration breakage.
    • Mitigation: run affected unit tests plus test/integration/event-log.test.ts; dogfood replay/export flows with an isolated home.

Generated with mux • Model: openai:gpt-5.5 • Thinking: xhigh

@ThomasK33
Copy link
Copy Markdown
Member Author

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean refactor. The codec extraction is well-scoped: single definition of MAX_EVENT_LOG_SIZE, JSONL parsing, EventRecordSchema validation, and contiguous sequence checks. Dependency graph stays acyclic (eventLogCodec imports only protocol/schemas and util/assert). Test coverage is thorough (228 lines of test for 93 lines of codec). CI green.

One concern, one nit. Both are stale documentation in AGENTS.md that now points developers to the wrong files. The full review panel has not yet reviewed this PR; these are mechanical findings from Netero's first pass. The panel will review after these are addressed.

"A developer following this instruction would search two files that no longer contain the limit, miss the actual source of truth, and potentially re-introduce the duplication this PR eliminated." (Netero)


AGENTS.md:160

P2 [DEREM-1] This instruction is now factually wrong. MAX_EVENT_LOG_SIZE no longer lives in src/host/eventLog.ts or src/host/replay.ts; it is defined exclusively in src/storage/eventLogCodec.ts. A developer following this text would search the wrong files and could re-introduce the duplication this PR eliminated.

Line 159 ("Keep src/host/eventLog.ts and src/host/replay.ts assumptions aligned") is also misleading: alignment is now enforced automatically by both modules delegating to the shared codec.

Both lines need updating to point to src/storage/eventLogCodec.ts as the single owner of event-log size limits, JSONL parsing, and sequence validation. (Netero)

🤖

AGENTS.md:45

P3 [DEREM-2] This description lists "path guards, home/session resolution, manifest I/O, and artifact manifests" but src/storage/ now also contains eventLogCodec.ts, which owns persisted event-log reading, validation, and the size limit constant. A developer navigating by this description would not know to look here for codec behavior.

Could we append "event-log codec" or similar? (Netero)

🤖

🤖 This review was automatically generated with Coder Agents.

@ThomasK33
Copy link
Copy Markdown
Member Author

Addressed DEREM-1 and DEREM-2 in 6675648 by updating AGENTS.md to point event-log size/parsing/schema/sequence ownership at src/storage/eventLogCodec.ts.

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel reviewed (12 reviewers). 9 of 12 found no issues. Clean refactor: single definition site for MAX_EVENT_LOG_SIZE, JSONL parsing, schema validation, and contiguous-sequence checks. Dependency graph stays acyclic. Behavioral fidelity verified across all callsites. 60+ tests pass, typecheck clean, CI green. Dogfood proof bundle exercises the full persisted-offline-replay path.

Two P3s and one P4, all test/documentation improvements. Nothing blocking.

"The codec has a tight dependency boundary. The readonly unknown[] signature on validateEventRecords forces callers to re-earn the EventRecord type through validation, which is the right defense-in-depth choice for a canonical codec." (Kite)

🤖 This review was automatically generated with Coder Agents.

Comment thread test/unit/storage/eventLogCodec.test.ts
Comment thread src/storage/eventLogCodec.ts
Comment thread test/unit/storage/eventLogCodec.test.ts
@ThomasK33
Copy link
Copy Markdown
Member Author

Addressed DEREM-3, DEREM-4, and DEREM-5 in d308a76. Replied inline and resolved the review threads.

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review panel (4 reviewers). All five prior findings verified fixed across three commits. No new findings from Netero or the panel. Zero open items.

All fixes addressed root causes: R1 rewrote stale architecture instructions (not just paths), R2 added the missing test assertions and doc comments at the exact locations identified. The d308a76 fix commit is 15 lines with no scope drift.

62 unit tests and 2 integration tests pass. Typecheck clean. CI green. Dogfood bundle exercises the full offline-replay path. Clean to merge.

"The plan-to-delivery gap is zero. The implementation plan's 5 phases and 12 acceptance criteria map 1:1 to the final diff." (Mafu-san)

🤖 This review was automatically generated with Coder Agents.

@ThomasK33 ThomasK33 merged commit 69271e7 into main Apr 29, 2026
11 checks passed
@ThomasK33 ThomasK33 deleted the grill-docs-cyte branch April 29, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract the event log codec used by append and replay

1 participant